Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display figures on a new github pages site. #49

Closed
wants to merge 6 commits into from

Conversation

AhmedBasem20
Copy link
Contributor

Describe the changes you have made in this PR

Displayed the generated plots from the analysis workflow to a new github-pages site.

Results

Example from my fork: https://ahmedbasem20.github.io/TF2.4_IVIM-MRI_CodeCollection/

@etpeterson To enable this on this repository we need the following steps:

  1. create a new branch named gh-pages.
  2. From the pages section on the repository settings, set gh-pages as a source branch.
image

Link this PR to an issue [optional]

Fixes #46

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@AhmedBasem20 AhmedBasem20 requested a review from etpeterson March 2, 2024 10:36
@etpeterson
Copy link
Contributor

This all looks good but I'm wondering how this actually works on the repository. We just went through a big cleaning process and we don't want to have to deal with that again, so I'm hyper vigilant about uploading things to the repository. It seems like this pushes the pdfs to the repository each time. I see this discussion about that, for example. That then points to this as a way to push but not keep the history.

In short, I think whatever we do, we don't want the history of the pdfs pushed, just the most recent.

.github/workflows/analysis.yml Outdated Show resolved Hide resolved
@AhmedBasem20
Copy link
Contributor Author

Thanks @etpeterson for the review, I used the deployment method you suggested and it worked fine, see: https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/8139256741

@etpeterson
Copy link
Contributor

etpeterson commented Mar 5, 2024

Thanks @etpeterson for the review, I used the deployment method you suggested and it worked fine, see: https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/8139256741

This run only ran 4 algorithms but I see on the hosted site that all of them ran. Why this discrepancy? I don't see anything in the code that should be changing that.

I also see there are a few pushes that caused commits but I don't see any changes more recently. I'm not sure how this should act, but with the algorithm discrepancy I'm still hesitant to merge until it's all clear.
image

@AhmedBasem20
Copy link
Contributor Author

My bad. I wanted to see the results quickly, so I removed most of the algorithms on this workflow. But the workflow of the committed code on this pull request works fine: https://github.com/AhmedBasem20/TF2.4_IVIM-MRI_CodeCollection/actions/runs/8139531990

I also see there is a few pushes that caused commits, but I don't see any changes more recently. I'm not sure how this should act, but with the algorithm discrepancy I'm still hesitant to merge until it's all clear.

Those pushes caused by the old action that deploys to gh-pages. It is removed now.

Copy link
Contributor

@etpeterson etpeterson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, just one small comment. It is a little opaque to me how the publishing works though. Have you found a good resource?

Perhaps another change would be however we should structure the website. I'm imagining a base page and we link to the results from there, or perhaps they're on the base page somewhere. Right now it's just the results page, correct? Maybe the comment in the related issue will get answered to help here.

.github/workflows/analysis.yml Show resolved Hide resolved
@AhmedBasem20
Copy link
Contributor Author

@etpeterson the publishing action depends on the artifact we uploaded, I used this action as a reference.
Will the documentation page be on this website? If so, we build this page along with the docs and add it there. But for now, I can improve the layout of the current HTML page until we get the documentation.

@AhmedBasem20 AhmedBasem20 requested a review from etpeterson March 6, 2024 14:00
@etpeterson
Copy link
Contributor

This looks good. I think there's an open question about where the results should land and if the index.html page should start with a little more on there, but that's for @petravanhoudt and @oliverchampion.

@etpeterson
Copy link
Contributor

Just following up @oliverchampion or @petravanhoudt if you have ideas on the structure of the page.

@AhmedBasem20
Copy link
Contributor Author

superseded by #59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Push Algorithm Analysis result plots to a static github page
2 participants